Skip to content

fix(server): run editor discovery concurrently and cache the result - #5050

Open
capad-xyz wants to merge 3 commits into
pingdotgg:mainfrom
capad-xyz:fix/concurrent-cached-editor-discovery
Open

fix(server): run editor discovery concurrently and cache the result#5050
capad-xyz wants to merge 3 commits into
pingdotgg:mainfrom
capad-xyz:fix/concurrent-cached-editor-discovery

Conversation

@capad-xyz

@capad-xyz capad-xyz commented Jul 30, 2026

Copy link
Copy Markdown

Problem

server.getConfig blocks on external editor discovery, which probes ~20 editor commands sequentially, each walking every PATH entry against every PATHEXT variant. On hosts with long PATHs this exceeds the 5s EDITOR_DISCOVERY_TIMEOUT on every call, so discovery is interrupted, callers get an empty editor list, and the next call starts over from scratch. Every client connection pays the full 5s on its first RPC.

Fixes #4210. Also addresses the empty-list symptom in #4697.

Fix

Two changes in ExternalLauncher:

  1. Probe concurrently. Effect.forEach(..., { concurrency: EDITORS.length }) instead of a sequential loop. Wall time becomes the slowest single probe rather than the sum.
  2. Run discovery in a detached fiber and let callers join it. Effect.cached around Effect.forkDetach, so callers share one scan via Fiber.join.

The second point is deliberate. Caching the scan directly is unsafe here: Effect.cached is cachedWithTTL(self, Duration.infinity), and cachedInvalidateWithTTL memoizes the exit through onExit(self, ...) — including an interrupted exit — and with an infinite TTL never recomputes. A caller hitting the 5s timeout mid-scan would pin that interrupted exit for the life of the process. Caching the fork instead memoizes a fiber handle, which is produced instantly and cannot carry an interrupted exit; a caller that times out cancels only its own wait while the scan runs to completion in the background. On a #4210-class machine (15-99s scans) editors now resolve on a subsequent getConfig instead of never.

EDITOR_DISCOVERY_TIMEOUT in ws.ts is untouched and still bounds the cold call.

Results are unchanged. Effect.forEach preserves input order, and per-editor command preference (resolveAvailableCommand) is untouched, so the returned list is identical to what the sequential loop produced — same editors, same order. There are no per-editor timeouts, so a slow-but-present editor cannot be dropped from the result.

Relation to #4739

#4739 targeted the same issue and was closed unmerged. This takes a different approach: it adds caching rather than only bounding latency, and it avoids per-editor timeouts, which is what changed selection behaviour there. Happy to close this if you would rather solve it another way — the measurements below may still be useful for whatever lands.

Measurements

Windows 11, 46-entry PATH, 12-entry PATHEXT, desktop nightly 0.0.32-nightly.20260730.958 (patched bundle):

before after
ws.rpc.server.getConfig 5,000-5,004 ms on every call, discovery always interrupted 4,789 ms once (cold, completes), then 0-1 ms
externalLauncher.buildAvailableEditors 5,000 ms Interrupted, every time 4,787 ms Success once, then cached

Discovery now completes, so clients receive a real editor list instead of a permanently empty one.

Tests

vp test run src/process/externalLauncher.test.ts — 6 passed. Two added:

  • discovery is cached per service instance
  • discovery survives a caller giving up: a first caller is interrupted mid-scan, and a later call must still resolve. Fails on the pre-fix commit with All fibers interrupted without error.

Not covered

This does not address the Android store app disconnect (#4901); that reproduces with a fast getConfig too and is protocol skew on the client build. This only removes the server-side stall.

Diagnosed and implemented with Claude Code (Fable 5).


Note

Medium Risk
Changes async/caching behavior on a hot RPC path (getConfig); logic is localized to ExternalLauncher with tests, but discovery can still be slow on the first cold call and cached results won't reflect PATH changes until process restart.

Overview
External editor discovery no longer blocks server.getConfig for the full sequential PATH scan on every call. buildAvailableEditors now probes all editors in parallel (Effect.forEach with full concurrency), and ExternalLauncher memoizes discovery by caching a detached fiber (Effect.cached + forkDetach + Fiber.join) so callers share one scan and a timed-out waiter does not poison later results.

Editor list order and membership stay the same as the old sequential loop. Tests add coverage for per-instance caching and for recovery when an earlier caller is interrupted mid-discovery.

Reviewed by Cursor Bugbot for commit 61278be. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fcbdc8e9-b5de-448e-bc59-9b6666f3b913

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jul 30, 2026
Comment thread apps/server/src/process/externalLauncher.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR introduces caching and concurrent execution for editor discovery, changing state management and interruption semantics. While tests are included, the new caching layer and detached fiber pattern represent meaningful runtime behavior changes that warrant human review.

You can customize Macroscope's approvability policy. Learn more.

capad-xyz and others added 2 commits July 31, 2026 04:26
Effect.cached memoizes the inner effect's exit, so an interrupted first
scan poisoned the cache permanently. Cache the fork instead and let
callers join a detached fiber, so a timed-out caller cancels only its
own wait.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Provider page stays on "Checking provider status" because server.getConfig blocks on editor discovery

1 participant